Draft: Sync pause and resume #951
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #951 +/- ##
===========================================
- Coverage 100.00% 98.81% -1.19%
===========================================
Files 54 54
Lines 1401 1430 +29
Branches 321 331 +10
===========================================
+ Hits 1401 1413 +12
- Misses 0 11 +11
- Partials 0 6 +6 ☔ View full report in Codecov by Sentry. |
|
Thanks for the PR @obedm503! 🙂 Some quick thoughts: My suggestion would be to keep |
| @@ -165,7 +160,11 @@ export default class SyncManager< | |||
| type: 'insert', | |||
| data: item, | |||
| }) | |||
| this.schedulePush(options.name) | |||
|
|
|||
| const isSyncing = this.getCollection(options.name)[2] | |||
There was a problem hiding this comment.
To avoid ambiguity with the isSyncing() method of the class, I would prefer change this maybe to isPaused and negate the logic.
| @@ -242,7 +236,7 @@ export default class SyncManager< | |||
| * @param options.force If true, the sync process will be started even if there are no changes and onlyWithChanges is true. | |||
| * @param options.onlyWithChanges If true, the sync process will only be started if there are changes. | |||
| */ | |||
| public async sync(name: string, options: { force?: boolean, onlyWithChanges?: boolean } = {}) { | |||
| private async sync(name: string, options: { force?: boolean, onlyWithChanges?: boolean } = {}) { | |||
There was a problem hiding this comment.
I think it's better to leave the sync method as it is and just toggle the pause status of the collection with the startSync/pauseSync methods. Otherwise it isn't possible to trigger a manual sync while the remote change listener is paused.
Why Is This Useful
The
SyncManageris an incredibly powerful addition to SignalDB but it is missing the ability to gracefully pause and resume sync.I maintain a large application made up of many independent modules. Each module uses many collections: some shared, some unique. This application is long-lived. Users leave it running for hours or days at a time. Users have permission to access some or all the modules. All modules are not equally important to the user depending on their role. Users mostly use a few modules and rarely use the rest. Some modules are only used on an annual or quarterly basis.
This change is useful because it allows us to build applications where collections can become active or inactive throughout the lifetime of the session as the user navigates around the application. It would allow us to save both network and server resources by not syncing collections while they're not in use.
Summary of Changes
This is my implementation of sync pause and resume. This PR would introduce the following changes:
registerRemoteChangetakes thecollectionOptionsjust likepushandpullregisterRemoteChangechanges from executing once in the constructor to executing for each collection every time sync resumesregisterRemoteChangecan return a clean-up function to allow for reconnection after a pause (think WebSockets)syncAlltostartSyncAll.startSyncAllfulfills the same purpose except it can be called multiple timesstartSyncwhich callsregisterRemoteChangeandsync. This effectively replacessync.syncis now private. Collections start in offline mode and only start syncing afterstartSyncis calledpauseSyncto clean up remote-change subscriptions and put the collection into offline mode.pauseSyncAllto pause sync for all collectionsFor an example of how this change would affect an application please see this diff: obedm503/trellix-offline@90b0300
Practical Example
The initial state of the application:
The application goes offline, web sockets are closed:
The user makes changes while offline but sync is paused:
The application goes back online, web sockets are reopened, and collections sync:
This is very much a draft PR. I know you initially said you're not sure about changing how remote-change subscriptions happen, but I hope I made a good case for the change. I updated the tests so they all pass. I have not updated the documentation but will do so once this is a final PR. Also, I tried to maintain your code style, but please let me know if there are any style changes you wish to make. For the future, I recommend adopting something like Prettier to enforce your style in an automated way.
Related to #947